Skip to content

notebooks updated - please check zenith angle calcs, strange values #78

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 10 commits into from
Closed

Conversation

NelisW
Copy link

@NelisW NelisW commented Jul 12, 2015

All notebooks run to completion without breaking, but please note:

  1. Zenith angles appear suspect, airmass returnd from atmosphere.airmass_absolute() are mostly NaN. Can someone best acquanted with zenith angle algorithms please investigate. I added some print code marked with #cjwcjw (my initials) to help with finding.
  2. Notebooks updated to provide correct named function parameters when calling functions, but the local names & column headers in the notebooks are still as original. Ideally we should update local names to be similar to function parameter names.
  3. No attempt was made to check the correctness of the results, I have to little experience in this field to recognise right from wrong. Can someone just please run the notebooks (they should work now) and check the results?
  4. No attempts at additional documentation & text. It hurts me to commit this :-( but I would rather commit this now than to wait another week.
  5. Minor adjustments were made to the code. The most important is in pvsystem.retrieve_sam() to download and save model files locally. You can now download once and the use the local file afterward.

go well!

@wholmgren
Copy link
Member

Oh crap I think there was a misunderstanding. I tried to say in #74 that all of the tutorials in the pull request were now working. That's a lot of duplicated tedious work. I'm really sorry about not making that more clear. I will compare your new notebooks to my new notebooks and we can figure out how to move forward.

Most of the changes that are not notebook related should go into a separate PR. Adding the ability to save SAM files via the retrieve_sam function is interesting but will need to be discussed.

I briefly looked at your notebooks that show zenith and airmass calculations. The calculations seemed ok to me: zenith angles are between 0 and 180 degrees, and airmass is only defined for zenith angles less than 90 degrees. What's not ok is that I accidentally made a plot (atmosphere cell 6) that labels airmass as zenith.

@bmu bmu mentioned this pull request Jul 12, 2015
9 tasks
@NelisW
Copy link
Author

NelisW commented Jul 13, 2015

Will & Co

no problem, I should have read more carefully. I think it is easiest if
you just ignore my changes, yours are probably more comprehensive. I have
to apologise for not bringing my contribution in time.

Not related to this matter, I must say goodbye. It became clear to me over
the weekend that personal obligations do not allow me to make a significant
contribution to this very exciting project. I just cannot make enough time
available without sacrificing in other areas. Perhaps later....

thanks for your warm and friendly attitude, this is a really nice team and
I really appreciate your ettiquete and kindness!

Go well and good luck.

On 12 July 2015 at 21:52, Will Holmgren [email protected] wrote:

Oh crap I think there was a misunderstanding. I tried to say in #74
#74 that all of the tutorials
in the pull request were now working. That's a lot of duplicated tedious
work. I'm really sorry about not making that more clear. I will compare
your new notebooks to my new notebooks and we can figure out how to move
forward.

Most of the changes that are not notebook related should go into a
separate PR. Adding the ability to save SAM files via the retrieve_sam
function is interesting but will need to be discussed.

I briefly looked at your notebooks that show zenith and airmass
calculations. The calculations seemed ok to me: zenith angles are between 0
and 180 degrees, and airmass is only defined for zenith angles less than 90
degrees. What's not ok is that I accidentally made a plot (atmosphere cell
6) that labels airmass as zenith.


Reply to this email directly or view it on GitHub
#78 (comment).

@wholmgren
Copy link
Member

@NelisW I'm sorry to see you go and I hope we can work together again in the future!

@wholmgren wholmgren closed this Jul 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants